-
Notifications
You must be signed in to change notification settings - Fork 60
feat: Add BTC signet support #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds Unisat Bitcoin wallet integration and Bitcoin Signet support: new UnisatWalletProvider and hook, BTC added to supported chains, NetworkSelector and UI threaded with chain-selection callback, and BTC PSBT build/sign/broadcast flow integrated into the call handler. App is wrapped with the provider. Changes
Sequence DiagramsequenceDiagram
participant User
participant NetworkSelector
participant Unisat as UnisatWalletProvider
participant AppHandler as useHandleCall
participant Signet as Signet Network
User->>NetworkSelector: Select Bitcoin chain
NetworkSelector->>Unisat: connect() / switchChain(BITCOIN_SIGNET)
Unisat->>User: Prompt approve (requestAccounts)
Unisat->>NetworkSelector: Return accounts & chain
User->>AppHandler: Initiate BTC transaction
AppHandler->>Unisat: ensure paymentAccount & getChain()
Unisat->>AppHandler: paymentAccount + chain info
rect rgb(220,235,255)
note right of AppHandler: PSBT flow (build → sign → finalize)
AppHandler->>AppHandler: Build PSBT
AppHandler->>Unisat: signPSBT(psbt)
Unisat->>User: Prompt sign PSBT
Unisat->>AppHandler: signed PSBT (base64)
end
AppHandler->>Signet: Broadcast signed TX
Signet->>AppHandler: TxID / result
AppHandler->>User: Notify result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🔇 Additional comments (1)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment @cursor review or bugbot run to trigger another review on this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/hello/frontend/src/ConnectedContent.tsx (1)
24-28:DynamicConnectedContentshould receive and invoke theonChainSelectcallback to maintain consistency withEip6963ConnectedContent.The
ConnectedContentPropsinterface definesonChainSelectas an optional prop (line 21), whichEip6963ConnectedContentreceives and respects (line 89). However,DynamicConnectedContentneither destructures this prop (lines 24-28) nor invokes it withinhandleNetworkSelect(lines 46-54). The parent component passesonChainSelectonly toEip6963ConnectedContent(line 147), not toDynamicConnectedContent(lines 137-141), creating inconsistent callback propagation between wallet implementations.Update
DynamicConnectedContentto destructure and invoke the callback:const DynamicConnectedContent = ({ selectedProvider, supportedChain, primaryWallet, + onChainSelect, }: ConnectedContentProps) => { const { switchChain } = useDynamicSwitchChainHook(); const userWallets = useUserWallets(); const switchWallet = useSwitchWallet(); const primaryWalletChain = primaryWallet?.chain; const walletIds: Record<string, string> = useMemo(() => { const solanaWallet = userWallets.find( (wallet) => wallet.chain === 'SOL' )?.id; const evmWallet = userWallets.find((wallet) => wallet.chain === 'EVM')?.id; return { EVM: evmWallet || '', SOL: solanaWallet || '', }; }, [userWallets]); const handleNetworkSelect = (chain: SupportedChain) => { + onChainSelect?.(chain); if (chain.chainType !== primaryWalletChain) { switchWallet(walletIds[chain.chainType]); } switchChain(chain.chainId); };Also update the call site (line 137-141):
<DynamicConnectedContent selectedProvider={selectedProvider} supportedChain={supportedChain} primaryWallet={primaryWallet} + onChainSelect={onChainSelect} />
🧹 Nitpick comments (2)
examples/hello/frontend/src/hooks/useHandleCall.ts (2)
321-321: Hard-coded gateway address should be configurable.The Signet gateway address
'tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur'is hard-coded. This makes it difficult to switch between environments (testnet/mainnet) and violates the DRY principle if the address is used elsewhere.Move the gateway address to a configuration file or constants:
// In constants/bitcoin.ts export const BITCOIN_GATEWAY_ADDRESSES = { mainnet: 'bc1q...', signet: 'tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur', testnet: 'tb1q...', } as const;Then reference it in the code:
await handleBitcoinCall( receiver, message, unisatWallet, - 'tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur', + BITCOIN_GATEWAY_ADDRESSES.signet, callbacks );
138-146: Network validation only supports Signet, but lacks clear error message.The code validates that the user is connected to
BITCOIN_SIGNETbut doesn't inform the user which network they should switch to or what networks are supported.Improve the error message:
if (chain.enum !== 'BITCOIN_SIGNET') { - throw new Error('Unisat wallet is not connected to Signet'); + throw new Error( + `Unisat wallet is connected to ${chain.name}, but Bitcoin Signet is required. ` + + 'Please switch to Bitcoin Signet in your Unisat wallet.' + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
examples/hello/frontend/src/App.tsx(1 hunks)examples/hello/frontend/src/ConnectedContent.tsx(4 hunks)examples/hello/frontend/src/Eip6963AppContent.tsx(2 hunks)examples/hello/frontend/src/components/NetworkSelector.tsx(3 hunks)examples/hello/frontend/src/constants/chains.ts(2 hunks)examples/hello/frontend/src/context/UnisatWalletProvider.tsx(1 hunks)examples/hello/frontend/src/hooks/useHandleCall.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T20:09:36.410Z
Learnt from: hernan-clich
PR: zeta-chain/example-contracts#280
File: examples/hello/frontend/src/App.tsx:8-9
Timestamp: 2025-09-17T20:09:36.410Z
Learning: In the zeta-chain/example-contracts repository, commit c9e419c6c8d6f045b06b2ba94c3c9f9b4ab05d0f addressed a runtime issue by splitting AppContent.tsx into DynamicAppContent.tsx and Eip6963AppContent.tsx components, providing better separation of concerns between dynamic wallet and EIP-6963 wallet implementations.
Applied to files:
examples/hello/frontend/src/Eip6963AppContent.tsxexamples/hello/frontend/src/App.tsx
🧬 Code graph analysis (6)
examples/hello/frontend/src/components/NetworkSelector.tsx (4)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
useUnisatWallet(288-296)examples/hello/frontend/src/components/Dropdown.tsx (1)
DropdownOption(5-12)examples/hello/frontend/src/constants/chains.ts (2)
SupportedChain(1-8)SUPPORTED_CHAINS(10-81)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)
examples/hello/frontend/src/Eip6963AppContent.tsx (2)
examples/hello/frontend/src/hooks/useEip6963Wallet.ts (1)
useEip6963Wallet(5-5)examples/hello/frontend/src/constants/chains.ts (2)
SUPPORTED_CHAINS(10-81)SupportedChain(1-8)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)
examples/hello/frontend/src/hooks/useHandleCall.ts (2)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
useUnisatWallet(288-296)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)
examples/hello/frontend/src/ConnectedContent.tsx (2)
examples/hello/frontend/src/constants/chains.ts (1)
SupportedChain(1-8)examples/hello/frontend/src/hooks/useSwitchChain.ts (1)
useSwitchChain(6-17)
examples/hello/frontend/src/App.tsx (5)
examples/hello/frontend/src/hooks/useTheme.ts (1)
useTheme(5-11)examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
UnisatWalletProvider(277-285)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)examples/hello/frontend/src/components/Header.tsx (1)
Header(52-54)examples/hello/frontend/src/AppContent.tsx (1)
AppContent(5-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (3)
examples/hello/frontend/src/constants/chains.ts (1)
72-80: PR title indicates mainnet support but implementation uses Signet testnet.The PR is titled "feat: Add BTC mainnet support", but the added chain is Bitcoin Signet (testnet). The chain ID
18333and explorer URLhttps://mempool.space/signet/tx/confirm this is a testnet configuration.If mainnet support is the actual goal, the configuration should be:
{ explorerUrl: (txHash: string) => `https://mempool.space/tx/${txHash}`, name: 'Bitcoin Mainnet', chainId: 1, // or use a different identifier for Bitcoin mainnet chainType: 'BTC', icon: '/logos/bitcoin-logo.svg', colorHex: '#F7931A', }Otherwise, update the PR title to reflect testnet/Signet implementation.
examples/hello/frontend/src/App.tsx (1)
12-26: LGTM! Proper provider integration.The
UnisatWalletProvideris correctly positioned as the outermost wrapper, ensuring the Unisat wallet context is available to all child components regardless of theUSE_DYNAMIC_WALLETflag.examples/hello/frontend/src/ConnectedContent.tsx (1)
93-98: Appropriate conditional chain switching for EVM only.The logic correctly invokes
onChainSelectfor all chain types while only performing automatic wallet chain switching for EVM chains. This aligns with the broader pattern where BTC chains require explicit Unisat wallet interaction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
examples/hello/frontend/src/hooks/useHandleCall.ts (1)
122-213: Remove console.log statements exposing sensitive transaction data.Lines 163, 175, 189-190, 202, and 210 contain console.log statements that expose sensitive transaction data (addresses, PSBTs, transaction IDs) to the console. These should be removed or gated behind a development-only flag.
Apply this diff to gate logs behind environment checks:
+const isDevelopment = import.meta.env.DEV; + async function handleBitcoinCall( ... ): Promise<void> { ... - console.log('Commit PSBT built:', commitResult); + if (isDevelopment) console.log('Commit PSBT built:', commitResult); ... - console.log('Commit PSBT signed, broadcasting...'); + if (isDevelopment) console.log('Commit PSBT signed, broadcasting...'); ... - console.log('Commit broadcasted:', revealResult.commitTxid); - console.log('Signing reveal PSBT...'); + if (isDevelopment) { + console.log('Commit broadcasted:', revealResult.commitTxid); + console.log('Signing reveal PSBT...'); + } ... - console.log('Reveal PSBT signed, broadcasting...'); + if (isDevelopment) console.log('Reveal PSBT signed, broadcasting...'); ... - console.log('Reveal broadcasted:', finalResult.revealTxid); + if (isDevelopment) console.log('Reveal broadcasted:', finalResult.revealTxid);examples/hello/frontend/src/components/NetworkSelector.tsx (1)
56-68: Hard-coded network limits Bitcoin network extensibility.Line 60 hard-codes
'BITCOIN_SIGNET'rather than deriving the network fromoption.value. While this works for the single BTC chain currently supported, it breaks extensibility when adding Bitcoin Mainnet, Testnet4, or other networks listed in theUnisatChaintype.Apply this diff to derive the network from the selected chain:
+// Map chain names to Unisat network enum +const chainToUnisatNetwork: Record<string, 'BITCOIN_SIGNET' | 'BITCOIN_MAINNET' | 'BITCOIN_TESTNET' | 'BITCOIN_TESTNET4'> = { + 'Bitcoin Signet': 'BITCOIN_SIGNET', + // Add other networks as they're supported +}; + const handleSelect = async (option: DropdownOption<SupportedChain>) => { if (option.value.chainType === 'BTC') { try { await connectUnisatWallet(); - await switchUnisatChain('BITCOIN_SIGNET'); + const unisatNetwork = chainToUnisatNetwork[option.value.name]; + if (!unisatNetwork) { + throw new Error(`Unsupported Bitcoin network: ${option.value.name}`); + } + await switchUnisatChain(unisatNetwork); } catch (error) { console.error('Failed to connect/switch Unisat wallet:', error); return; // Don't update selection if connection failed } } onNetworkSelect?.(option.value); };
🧹 Nitpick comments (1)
examples/hello/frontend/src/hooks/useHandleCall.ts (1)
305-311: Hard-coded Signet gateway address limits network flexibility.Line 309 hard-codes the Signet gateway address. When extending to Bitcoin Mainnet or other networks, this address must change. Consider deriving the gateway address from the chain configuration or environment variables.
+// Add to constants/chains.ts or environment config +const BITCOIN_GATEWAY_ADDRESSES: Record<string, string> = { + 'Bitcoin Signet': 'tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur', + // 'Bitcoin Mainnet': '<mainnet-gateway-address>', +}; + await handleBitcoinCall( receiver, message, unisatWallet, - 'tb1qy9pqmk2pd9sv63g27jt8r657wy0d9ueeh0nqur', // Signet gateway address + BITCOIN_GATEWAY_ADDRESSES[supportedChain.name] || + (() => { throw new Error(`No gateway address for ${supportedChain.name}`); })(), callbacks );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
examples/hello/frontend/src/Eip6963AppContent.tsx(2 hunks)examples/hello/frontend/src/components/NetworkSelector.tsx(3 hunks)examples/hello/frontend/src/context/UnisatWalletProvider.tsx(1 hunks)examples/hello/frontend/src/hooks/useHandleCall.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-17T15:57:52.805Z
Learnt from: hernan-clich
PR: zeta-chain/example-contracts#297
File: examples/hello/frontend/src/Eip6963AppContent.tsx:19-24
Timestamp: 2025-10-17T15:57:52.805Z
Learning: In the zeta-chain/example-contracts repository's Eip6963AppContent.tsx, the useEffect synchronization pattern `if (walletChain?.chainType === 'EVM')` before calling `setSelectedChain(walletChain)` is intentional for multi-wallet architecture: it syncs selectedChain with EIP-6963 wallet state only for EVM chains, preventing automatic overwrites when users manually select non-EVM chains (BTC/SOL) via onChainSelect callback, since EIP-6963 wallets remain on EVM chains and don't natively switch to non-EVM chains.
Applied to files:
examples/hello/frontend/src/hooks/useHandleCall.tsexamples/hello/frontend/src/components/NetworkSelector.tsxexamples/hello/frontend/src/Eip6963AppContent.tsx
📚 Learning: 2025-09-17T20:09:36.410Z
Learnt from: hernan-clich
PR: zeta-chain/example-contracts#280
File: examples/hello/frontend/src/App.tsx:8-9
Timestamp: 2025-09-17T20:09:36.410Z
Learning: In the zeta-chain/example-contracts repository, commit c9e419c6c8d6f045b06b2ba94c3c9f9b4ab05d0f addressed a runtime issue by splitting AppContent.tsx into DynamicAppContent.tsx and Eip6963AppContent.tsx components, providing better separation of concerns between dynamic wallet and EIP-6963 wallet implementations.
Applied to files:
examples/hello/frontend/src/Eip6963AppContent.tsx
🧬 Code graph analysis (4)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)
examples/hello/frontend/src/hooks/useHandleCall.ts (2)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
useUnisatWallet(287-295)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)
examples/hello/frontend/src/components/NetworkSelector.tsx (4)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
useUnisatWallet(287-295)examples/hello/frontend/src/components/Dropdown.tsx (1)
DropdownOption(5-12)examples/hello/frontend/src/constants/chains.ts (2)
SupportedChain(1-8)SUPPORTED_CHAINS(10-81)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)
examples/hello/frontend/src/Eip6963AppContent.tsx (2)
examples/hello/frontend/src/hooks/useEip6963Wallet.ts (1)
useEip6963Wallet(5-5)examples/hello/frontend/src/constants/chains.ts (2)
SUPPORTED_CHAINS(10-81)SupportedChain(1-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/swap)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (15)
examples/hello/frontend/src/components/NetworkSelector.tsx (2)
1-24: LGTM: Clean Unisat wallet integration.The import and hook usage follow React best practices. The hook is called unconditionally at the top level, correctly accessing the Unisat wallet context.
27-43: LGTM: Clear chain filtering logic.The filtering strategy correctly separates Dynamic wallet chains (EVM/SOL) from EIP-6963 chains (EVM/BTC), with appropriate metadata mapping to dropdown options.
examples/hello/frontend/src/Eip6963AppContent.tsx (3)
1-10: LGTM: Appropriate imports for chain state management.The new imports (useEffect, useState, SupportedChain) support the local chain selection state required for multi-wallet architecture.
11-26: LGTM: Correct multi-wallet synchronization pattern.The EVM-only check at line 23 is intentional for multi-wallet architecture: it syncs
selectedChainwith EIP-6963 wallet state only for EVM chains, preventing automatic overwrites when users manually select non-EVM chains (BTC/SOL) via theonChainSelectcallback. The clear comments document this behavior.Based on learnings.
28-41: LGTM: Correct prop threading for chain selection.The props correctly pass
selectedChainandsetSelectedChainto ConnectedContent, enabling the manual chain selection callback flow for non-EVM chains.examples/hello/frontend/src/hooks/useHandleCall.ts (4)
1-46: LGTM: Appropriate imports for Bitcoin integration.The new Bitcoin toolkit imports and Unisat wallet hook support the PSBT-based Bitcoin flow. Type definitions remain clean and comprehensive.
219-234: LGTM: Correct unconditional hook usage.The
useUnisatWallethook is correctly called at the top level, with a clear comment explaining it's only actively used whenUSE_DYNAMIC_WALLETis false. This follows React's rules of hooks.
235-259: LGTM: Comprehensive wallet validation logic.The validation sequence correctly checks wallet address availability (including Unisat), supported chain existence, and wallet-chain type compatibility. The flow is logical and complete.
274-334: LGTM: Well-structured BTC integration with proper guards.The BTC branch correctly guards against Dynamic wallet mode (line 299) and validates Unisat payment account (line 302). The error handling is consistent across all chains, and the dependencies array correctly includes
unisatWallet.examples/hello/frontend/src/context/UnisatWalletProvider.tsx (6)
1-70: LGTM: Comprehensive type definitions with browser-compatible utilities.The import of ethers'
hexlifyandgetBytes(line 1) correctly addresses the Node Buffer compatibility issue raised in past comments. Type definitions are thorough and accurately model the Unisat wallet API.
72-105: LGTM: Clean stub implementation for Dynamic wallet mode.The stub provider correctly prevents Unisat-specific code from executing when
USE_DYNAMIC_WALLETis true, with clear error messages indicating the feature is unavailable in Dynamic wallet mode.
134-184: LGTM: Robust connection handling with proper cleanup.The connect flow provides clear user guidance when Unisat is unavailable (lines 137-141). The
disconnectcallback is correctly memoized withuseCallback, and theuseEffectproperly includes it in the dependency array, addressing the stale closure issue from past comments. Event listener cleanup is correct.
186-222: LGTM: Browser-compatible PSBT encoding with ethers utilities.The base64 ↔ hex conversions correctly use ethers'
hexlifyandgetBytes(lines 197-199, 213-215), resolving the Buffer compatibility issue from past comments. The format conversion fortoSignInputs(lines 202-204) correctly maps to Unisat's expected structure.
224-250: LGTM: Clean chain management wrappers.The
getChainandswitchChainmethods are clean wrappers around the Unisat provider, with appropriate null checks and error handling.
252-295: LGTM: Well-structured context provider with conditional rendering.The context value correctly derives
paymentAccountand appropriately excludesordinalsAccount(which would always be null given the current implementation). The conditional provider (lines 276-284) cleanly switches between stub and actual implementations based onUSE_DYNAMIC_WALLET. TheuseUnisatWallethook includes a proper runtime guard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
examples/hello/frontend/src/hooks/useHandleCall.ts (1)
166-166: Console.log statements still present.Multiple console.log statements exposing transaction data remain in the code (lines 166, 178, 192-193, 205, 213). These were previously flagged and reportedly addressed in commits f43e42d to bfc181e, but are still present in this version.
Also applies to: 178-178, 192-193, 205-205, 213-213
🧹 Nitpick comments (1)
examples/hello/frontend/src/hooks/useHandleCall.ts (1)
235-236: Clarify hook usage comment.The comment "will only be used when USE_DYNAMIC_WALLET is false" may be misleading. While the hook is correctly called unconditionally (per React rules), the returned
unisatWalletvalue is only utilized whenUSE_DYNAMIC_WALLETis false. Consider clarifying to avoid confusion.- // Always call the hook - will only be used when USE_DYNAMIC_WALLET is false + // Always call the hook (per React rules) - the wallet is only utilized when USE_DYNAMIC_WALLET is false const unisatWallet = useUnisatWallet();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
examples/hello/frontend/src/constants/chains.ts(2 hunks)examples/hello/frontend/src/hooks/useHandleCall.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/hello/frontend/src/constants/chains.ts
🧰 Additional context used
🧬 Code graph analysis (1)
examples/hello/frontend/src/hooks/useHandleCall.ts (3)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
useUnisatWallet(287-295)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)examples/hello/frontend/src/constants/chains.ts (1)
BITCOIN_GATEWAY_ADDRESS_SIGNET(83-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/nft)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (5)
examples/hello/frontend/src/hooks/useHandleCall.ts (5)
1-5: LGTM! Bitcoin imports properly structured.The new imports for Bitcoin toolkit functions and Unisat wallet integration are well-organized and necessary for the BTC support feature.
Also applies to: 13-18
145-149: Effective network validation.The Signet network validation ensures the Unisat wallet is connected to the correct Bitcoin network before proceeding with transactions, preventing cross-network errors.
239-244: Proper multi-wallet address resolution.The wallet address resolution correctly prioritizes
primaryWallet, falls back toaccount, and finally considersunisatWallet?.paymentAccount?.address, ensuring compatibility across different wallet types.
301-314: Well-structured BTC wallet type handling.The Bitcoin flow includes appropriate guards for Dynamic wallet incompatibility and Unisat wallet presence, with clear error messages and proper parameter passing to
handleBitcoinCall.
324-337: Dependency array correctly updated.The addition of
unisatWalletto the dependency array ensures thehandleCallcallback is properly regenerated when the Unisat wallet state changes, maintaining correct closure semantics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
examples/hello/frontend/src/hooks/useHandleCall.ts (2)
155-164: Critical: Missing receiver address validation.The
receiverparameter (described as "Universal Contract address (20 bytes)" at line 298) is passed directly tobuildBitcoinInscriptionCallCommitPsbtwithout validation. Invalid receiver addresses can cause transaction failures or potential fund loss.Add validation before building the PSBT:
const network = 'signet'; + // Validate receiver address format + if (!receiver || !/^0x[0-9a-fA-F]{40}$/.test(receiver)) { + throw new Error( + 'Invalid receiver address: must be 20-byte hex string (0x-prefixed, 42 characters)' + ); + } const commitResult = await buildBitcoinInscriptionCallCommitPsbt({
152-153: Configuration still hardcoded for Signet.The Bitcoin API URL and network remain hardcoded to Signet, preventing support for mainnet, testnet, or alternative API providers. This was previously flagged and should be addressed for production readiness.
Consider extracting these to environment variables or configuration constants:
+const BITCOIN_NETWORK = import.meta.env.VITE_BITCOIN_NETWORK || 'signet'; +const BITCOIN_API_URL = import.meta.env.VITE_BITCOIN_API_URL || 'https://mempool.space/signet/api'; + async function handleBitcoinCall( receiver: string, message: string, unisatWallet: ReturnType<typeof useUnisatWallet>, gatewayAddress: string, callbacks: { onSigningStart?: UseHandleCallParams['onSigningStart']; onTransactionSubmitted?: UseHandleCallParams['onTransactionSubmitted']; onTransactionConfirmed?: UseHandleCallParams['onTransactionConfirmed']; } ): Promise<void> { // ... - const bitcoinApi = 'https://mempool.space/signet/api'; - const network = 'signet'; + const bitcoinApi = BITCOIN_API_URL; + const network = BITCOIN_NETWORK;
🧹 Nitpick comments (2)
examples/hello/frontend/src/hooks/useHandleCall.ts (2)
177-202: Consider adding error context for PSBT operations.The PSBT broadcast and signing operations could fail for various reasons (network issues, insufficient funds, invalid signatures). Adding specific error context would improve debuggability.
Consider wrapping critical operations with contextual error handling:
try { const revealResult = await broadcastCommitAndBuildRevealPsbt({ bitcoinApi, commitData: commitResult.commitData, depositFee: commitResult.depositFee, fromAddress: paymentAccount.address, gatewayAddress, network, revealFee: commitResult.revealFee, signedCommitPsbtBase64: signedCommitPsbt, }); } catch (error) { throw new Error( `Failed to broadcast commit transaction: ${error instanceof Error ? error.message : 'Unknown error'}` ); }
228-233: Consider conditional wallet address precedence for BTC flows.The wallet address resolution uses Unisat as the final fallback. For BTC-specific flows, consider making the precedence explicit based on wallet type to improve clarity and prevent potential address mismatches.
const walletAddress = walletType === 'BTC' ? unisatWallet?.paymentAccount?.address : primaryWallet?.address || account || unisatWallet?.paymentAccount?.address;This makes the BTC-specific address resolution more explicit and prevents potential confusion when multiple wallet sources are available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
examples/hello/frontend/src/hooks/useHandleCall.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/hello/frontend/src/hooks/useHandleCall.ts (3)
examples/hello/frontend/src/context/UnisatWalletProvider.tsx (1)
useUnisatWallet(287-295)examples/hello/frontend/src/constants/wallets.ts (1)
USE_DYNAMIC_WALLET(2-2)examples/hello/frontend/src/constants/chains.ts (1)
BITCOIN_GATEWAY_ADDRESS_SIGNET(83-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/nft)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/hello)
- GitHub Check: test (examples/swap)
- GitHub Check: test (examples/call)
- GitHub Check: test (examples/token)
- GitHub Check: test (examples/nft)
- GitHub Check: slither (examples/call, call.sarif)
- GitHub Check: slither (examples/swap, swap.sarif)
- GitHub Check: slither (examples/hello, hello.sarif)
🔇 Additional comments (5)
examples/hello/frontend/src/hooks/useHandleCall.ts (5)
1-5: LGTM!The Bitcoin toolkit imports are correctly structured and provide the necessary functionality for the PSBT-based call flow.
13-18: LGTM!The additional imports properly support the Bitcoin integration by bringing in the necessary gateway address, wallet feature flag, and Unisat wallet hook.
224-225: LGTM!The unconditional hook call follows React rules, with conditional usage properly handled in the callback logic based on the USE_DYNAMIC_WALLET flag.
235-251: LGTM!The validation checks are comprehensive and correctly guard against missing wallet addresses, unsupported chains, and wallet-chain type mismatches. The duplicate validation issue from previous reviews has been addressed.
290-303: BTC flow integration looks good.The Bitcoin wallet type handling properly guards against the unsupported Dynamic wallet scenario, validates the Unisat payment account, and correctly invokes the Bitcoin-specific handler with appropriate parameters and callbacks.
Ensure that the receiver address validation suggested in the earlier comment is implemented before this code reaches production, as the comment at line 298 indicates it expects a specific 20-byte format that is not currently validated.
fadeev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
18333 → 7001 ✅ OutboundMined
CCTX: 0x9552725d047d8c82fceda306fdfbe162b3ece9c4f993720cfc339696adb0ae1a
Tx Hash: 2d1622fd4b64194e61af527502472a5c35ac6834ad7ccd9e9409eed1d11049bf (on chain 18333)
Tx Hash: 0xd859a1078fc2df3ae34e79db3cfe52575a3cff1e2f5c66b9616b8cb302e2b012 (on chain 7001)
Sender: tb1qw9tydqfkhcnl2f2m0m0xus3h90s9zd6zj7rhs8
Receiver: 0x61a184EB30D29eD0395d1ADF38CC7d2F966c4A82
Message: 0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000c4865792c204865726e616e210000000000000000000000000000000000000000
Amount: 136 Gas tokens
Todos
package.jsonWalkthrough
rec-btc-hello-1.mp4
Note
Adds BTC Signet support via Unisat wallet, integrates network selection and PSBT-based call flow, and wraps the app with a Unisat provider.
AppwithUnisatWalletProvider; stubbed whenUSE_DYNAMIC_WALLETis true.context/UnisatWalletProviderwith connect, account/state, chain switch, and PSBT signing helpers.SupportedChain.chainTypeto includeBTCand addBitcoin SignettoSUPPORTED_CHAINS.NetworkSelectorfilters chains to includeSOL(Dynamic) orBTC(EIP-6963) and, for BTC, triggers Unisat connect and switches toBITCOIN_SIGNET.onChainSelectto propagate manual chain selection; EIP-6963 only switches for EVM and delegates selection via callback.selectedChainlocally and sync with wallet chain for EVM; passonChainSelecttoConnectedContent.useHandleCalladds BTC path (Unisat + Signet): build/sign/broadcast commit & reveal PSBTs using toolkit; guarded whenUSE_DYNAMIC_WALLETis true; retains existing EVM/SOL flows.Written by Cursor Bugbot for commit c6cc53b. Configure here.
Summary by CodeRabbit
New Features
Chores